-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow nullable foreign keys #12
Conversation
djantic/main.py
Outdated
@@ -2,6 +2,7 @@ | |||
from enum import Enum | |||
from functools import reduce | |||
from itertools import chain | |||
from types import UnionType |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like UnionType doesn't exist in python 3.8 and 3.9 either.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are correct. After some reading, UnionType was introduced in 3.10, with the "|" annotation (e.g. int | str
).
I can make the tests compatible for python 3.8 by removing this syntax, but still supporting this check (with an import only if sys.version_info >= (3,10)
).
This means I can support this syntax, but not check for it in a test.
What do you suggest we do?
if sys.version_info >= (3, 10): | ||
from types import UnionType | ||
else: | ||
from typing import Union as UnionType |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@idan-david This looks ok to me. The import on line 6 needs to be removed though. Let's see if the tests pass after this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the mess, removed the import on line 6
(get_origin(annotation) is Union or get_origin(annotation) is UnionType) | ||
and type(None) in args | ||
and len(args) == 2 | ||
and any(inspect.isclass(arg) and issubclass(arg, ModelSchema) for arg in args) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was causing problems with python 3.9, added the inspect.isclass(arg)
@idan-david 🎉 thanks for taking the time |
Thank you for your patience |
In the case of a model with a nullable foreign key to another model the
from_django
method does not currently work.I've added a check for Optional types in the
dict
method, which extracts the correct type